Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump rails from 7.0.8 to 7.1.0 #5031

Merged
merged 19 commits into from
Oct 11, 2023
Merged

Bump rails from 7.0.8 to 7.1.0 #5031

merged 19 commits into from
Oct 11, 2023

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Oct 9, 2023

Bumps rails from 7.0.8 to 7.1.0.

Release notes

Sourced from rails's releases.

7.1.0

Active Support

  • Fix AS::MessagePack with ENV["RAILS_MAX_THREADS"].

    Jonathan Hefner

  • Add a new public API for broadcasting logs

    This feature existed for a while but was until now a private API. Broadcasting log allows to send log message to difference sinks (STDOUT, a file ...) and is used by default in the development environment to write logs both on STDOUT and in the "development.log" file.

    Basic usage:

    stdout_logger = Logger.new(STDOUT)
    file_logger = Logger.new("development.log")
    broadcast = ActiveSupport::BroadcastLogger.new(stdout_logger, file_logger)
    broadcast.info("Hello!") # The "Hello!" message is written on STDOUT and in the log file.

    Adding other sink(s) to the broadcast:

    broadcast = ActiveSupport::BroadcastLogger.new
    broadcast.broadcast_to(Logger.new(STDERR))

    Remove a sink from the broadcast:

    stdout_logger = Logger.new(STDOUT)
    broadcast = ActiveSupport::BroadcastLogger.new(stdout_logger)
    broadcast.stop_broadcasting_to(stdout_logger)

    Edouard Chin

  • Fix Range#overlap? not taking empty ranges into account on Ruby < 3.3

    Nobuyoshi Nakada, Shouichi Kamiya, Hartley McGuire

  • Use Ruby 3.3 Range#overlap? if available

    Yasuo Honda

... (truncated)

Commits
  • d39db5d Preparing for 7.1.0 release
  • 4cc7947 Merge pull request #49491 from tnir/tn-date-rails71
  • d206e8c Release notes are not in work in progress anymore
  • 60393bb Merge pull request #49493 from skipkayhil/hm-no-backticks
  • 9826678 Add corrected release month for Rails 7.1
  • 525b09c Add release month for Rails 7.1
  • b6842b2 Merge pull request #49391 from ipc103/transaction-tracking-on-reconnect
  • 311f639 Merge pull request #49487 from yawboakye/clarify-login-procedure-expectation
  • f05e283 Merge pull request #49484 from hachi8833/fix_heading_migration_guide
  • 4280a03 Merge pull request #49480 from hachi8833/add_doc_db_prepare
  • Additional commits viewable in compare view

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
  • @dependabot ignore <dependency name> major version will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself)
  • @dependabot ignore <dependency name> minor version will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself)
  • @dependabot ignore <dependency name> will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself)
  • @dependabot unignore <dependency name> will remove all of the ignore conditions of the specified dependency
  • @dependabot unignore <dependency name> <ignore condition> will remove the ignore condition of the specified dependency and ignore conditions

dependabot bot added 2 commits October 9, 2023 11:52
Bumps [ddtrace](https://github.com/DataDog/dd-trace-rb) from 1.14.0 to 1.15.0.
- [Release notes](https://github.com/DataDog/dd-trace-rb/releases)
- [Changelog](https://github.com/DataDog/dd-trace-rb/blob/master/CHANGELOG.md)
- [Commits](DataDog/dd-trace-rb@v1.14.0...v1.15.0)

---
updated-dependencies:
- dependency-name: ddtrace
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [rails](https://github.com/rails/rails) from 7.0.8 to 7.1.0.
- [Release notes](https://github.com/rails/rails/releases)
- [Commits](rails/rails@v7.0.8...v7.1.0)

---
updated-dependencies:
- dependency-name: rails
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
@dependabot dependabot bot requested a review from jorg-vr as a code owner October 9, 2023 11:56
@dependabot dependabot bot added the dependencies Pull requests that update a dependency file label Oct 9, 2023
@jorg-vr jorg-vr requested a review from a team as a code owner October 9, 2023 12:35
@jorg-vr jorg-vr requested review from niknetniko and removed request for a team October 9, 2023 12:35
@jorg-vr
Copy link
Contributor

jorg-vr commented Oct 9, 2023

Changes made by me:

  1. Removed the popularity enum from activity.
    I incorrectly used an enum here as in rails an enum is always linked to a database column. The fact that tests broke now is probably because Rails added more database validations before running tests.
    The enum was only used to get a list of all possible values, which I fixed by using the threshold map keys.

  2. fixed redirect tests
    2 tests expecting a redirect, did an odd check on the response body

  3. used correct path for a missing translation
    This missing translation was only reported now due to config.i18n.raise_on_missing_translations = true now raises on any missing translation.

@jorg-vr jorg-vr marked this pull request as draft October 9, 2023 14:07
@jorg-vr
Copy link
Contributor

jorg-vr commented Oct 10, 2023

Next steps applied by me:
I ran bin/rails app:update and tried to update our configs to the new defaults where relevant

I applied the new rails defaults one by one as specified here: 41635ce and then update the new default configs to 7.1

Notable changes:
I kept config.eager_load = false for tests as setting this true in CI causes lot's of failures. This should be investigated further as this might mean it will break in production.

I kept the default sanitizer on Rails::HTML4::Sanitizer instead of HTML5 because of this issue rails/rails-html-sanitizer#169

I had to use require_relative in result_constructor_test because the normal require was broken. I still do not understand why a manual require is even needed here as tests succeed without this require 4/5 times.
I also do not understand what breaks the normal require. None of the changes in 41635ce had any impact, but changing the default config to 7.1 did have an impact. This is odd as the 7.1 config should be functionally the same between having all individual configs active.

I have also not yet updated config.active_support.cache_format_version as this change is not revertible. SO this would better be suited for a separate pr in the future

Todo

  • investigate if eager loading is not broken on naos
  • create an issue to update config.active_support.cache_format_version

@jorg-vr jorg-vr added the deploy naos Request a deployment on naos label Oct 10, 2023
@jorg-vr jorg-vr temporarily deployed to naos October 10, 2023 13:33 — with GitHub Actions Inactive
@github-actions github-actions bot removed the deploy naos Request a deployment on naos label Oct 10, 2023
@jorg-vr jorg-vr marked this pull request as ready for review October 10, 2023 13:46
Copy link
Member

@niknetniko niknetniko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything seems to still work.

The require_relative is probably due to https://edgeguides.rubyonrails.org/upgrading_ruby_on_rails.html#autoloaded-paths-are-no-longer-in-$load-path.

Do we have an issue (or can we fix them now?) to track deprecation warnings, such as:

DEPRECATION WARNING: DeprecatedConstantAccessor.deprecate_constant without a deprecator is deprecated (called from <top (required)> at /home/niko/Ontwikkeling/dodona/config/application.rb:7)
DEPRECATION WARNING: Support for the pre-Ruby 2.4 behavior of to_time has been deprecated and will be removed in Rails 7.2. (called from <top (required)> at /home/niko/Ontwikkeling/dodona/config/initializers/new_framework_defaults.rb:17)
DEPRECATION WARNING: `Rails.application.secrets` is deprecated in favor of `Rails.application.credentials` and will be removed in Rails 7.2. (called from <top (required)> at /home/niko/Ontwikkeling/dodona/config/environment.rb:11)

config/application.rb Outdated Show resolved Hide resolved
@jorg-vr
Copy link
Contributor

jorg-vr commented Oct 11, 2023

I agree https://edgeguides.rubyonrails.org/upgrading_ruby_on_rails.html#autoloaded-paths-are-no-longer-in-$load-path is the cause of the change.
But if I understand it correctly no require should be needed at all because autoload fixes this? It is odd that the tests fail 1 out of 5 times without any require statement.

Do you prefer the current solution or adding config.add_autoload_paths_to_load_path = true to the test config?

@jorg-vr
Copy link
Contributor

jorg-vr commented Oct 11, 2023

I have solved:

DEPRECATION WARNING: Support for the pre-Ruby 2.4 behavior of to_time has been deprecated and will be removed in Rails 7.2. (called from <top (required)> at /home/niko/Ontwikkeling/dodona/config/initializers/new_framework_defaults.rb:17)

I just allowed the new setting. We only used to_time once, and even there it is immediately converted to UTC, so timezone does not seem to matter that much.

I haven't seen:

DEPRECATION WARNING: `Rails.application.secrets` is deprecated in favor of `Rails.application.credentials` and will be removed in Rails 7.2. (called from <top (required)> at /home/niko/Ontwikkeling/dodona/config/environment.rb:11)

I also do not see why we get this error as we already use the new credentials system instead of secrets. Could it be that you still have an old secrets file locally?

The last depecreation error is probably caused by one of our dependencies as it is mentioned when running Bundler.require. I have not identified the dependency that caused it. But we might just wait and hope the dependency will update when they see the error themselves.

DEPRECATION WARNING: DeprecatedConstantAccessor.deprecate_constant without a deprecator is deprecated (called from <top (required)> at /home/niko/Ontwikkeling/dodona/config/application.rb:7)

@jorg-vr jorg-vr requested a review from niknetniko October 11, 2023 12:03
Copy link
Member

@niknetniko niknetniko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if I understand it correctly no require should be needed at all because autoload fixes this? It is odd that the tests fail 1 out of 5 times without any require statement.

It is indeed a weird issue, as the failures are a bit flaky (although they do seem to happen a lot more if running tests in parallel than when running in a single process).

Do you prefer the current solution or adding config.add_autoload_paths_to_load_path = true to the test config?

I think the current solution is fine.

I did have a secrets file around, and as long as we don't cause deprecation warnings, no issue.

@jorg-vr jorg-vr requested a review from bmesuere October 11, 2023 12:21
Copy link
Member

@bmesuere bmesuere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add the changes from #5027 to this PR? This will simplify a potential rollback.

@jorg-vr jorg-vr merged commit 15e6d14 into main Oct 11, 2023
13 checks passed
@jorg-vr jorg-vr deleted the dependabot/bundler/rails-7.1.0 branch October 11, 2023 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants